Conversation
| @@ -42,46 +49,30 @@ export class DonationsController { | |||
| type: 'object', | |||
| properties: { | |||
| foodManufacturerId: { type: 'integer', example: 1 }, | |||
There was a problem hiding this comment.
NOTE FOR PR Reviewer: We deleted this since it does not make sense to have either of these fields here in creating a donation. The status will always be the same, as will the date donated.
dburkhart07
left a comment
dburkhart07
left a comment
There was a problem hiding this comment.
Leaving comment for PR reviewer
apps/frontend/src/containers/foodManufacturerDonationManagement.tsx
Outdated
Show resolved
Hide resolved
apps/frontend/src/containers/foodManufacturerDonationManagement.tsx
Outdated
Show resolved
Hide resolved
| }; | ||
| setCurrentPages(initialPages); | ||
| } catch (error) { | ||
| alert('Error fetching donations: ' + error); |
There was a problem hiding this comment.
might need to change depending on when justin's alert pr goes in
There was a problem hiding this comment.
will keep this comment unresolved till then.
| > | ||
| {parseInt(endsAfter) > 1 | ||
| ? 'Occurrences' | ||
| : 'Occurrence'} |
There was a problem hiding this comment.
wait instead of making the number as big as "occurrence" can you make them both smaller to match the rest of the fields
| import AdminOrderManagement from '@containers/adminOrderManagement'; | ||
| import { Amplify } from 'aws-amplify'; | ||
| import CognitoAuthConfig from './aws-exports'; | ||
| import { Button } from '@chakra-ui/react'; |
There was a problem hiding this comment.
seems to be some unnecessary imports
|
|
||
| @Get('/:donationId') | ||
| @Get('/by-fm-id/:foodManufacturerId') | ||
| async getDonationsByFoodManufacturer( |
There was a problem hiding this comment.
i think we can probably write controller tests
| display="inline-flex" | ||
| alignItems="center" | ||
| justifyContent="space-between" | ||
| backgroundColor={colors[0]} |
There was a problem hiding this comment.
i think there could be a tad more room above each status title
| textStyles, | ||
| tokens: { | ||
| colors: { | ||
| dashboardStatuses: { |
There was a problem hiding this comment.
i think if you look in the figma these have actual names within the design system that fit into colors, e.g. blue 100, yellow hover, etc. so can you switch to putting them in the color categories. also i think your color for available is too dark
| <Checkbox.Indicator /> | ||
| </Checkbox.Control> | ||
| <Checkbox.Label color="neutral.700"> | ||
| Make Donation Recurring |
There was a problem hiding this comment.
can we make this text not as bold?
| size="md" | ||
| fontWeight={600} | ||
| > | ||
| Submit Donation |
There was a problem hiding this comment.
i think this text should be white
| validateId(foodManufacturerId, 'Food Manufacturer'); | ||
|
|
||
| const manufacturer = await this.manufacturerRepo.findOne({ | ||
| where: { foodManufacturerId: foodManufacturerId }, |
There was a problem hiding this comment.
do we need two foodManufacturerId's here since they're named the same thing
| } | ||
| }; | ||
|
|
||
| const generateNextDonationDates = (): string[] => { |
There was a problem hiding this comment.
perhaps we could move this to a utils and write tests for it...
|
|
||
| <Table.Cell> | ||
| <Input | ||
| _placeholder={{ color: 'neutral.300' }} |
There was a problem hiding this comment.
can we move these stylings to a variable
3 participants
ℹ️ Issue
Closes #134
📝 Description
✔️ Verification
Took screenshots of the pages:
Verified each updated endpoint worked on Postman
🏕️ (Optional) Future Work / Notes
We will need to update this later on when we go to finish up the Action Required tab.